Bump `clap` to 3.0 #3064

zohnannor · 2022-08-21T22:56:01Z

Bump clap to v3 which fixes incorrect executable name printing in subcommand's help texts on Windows (it comes as rustup.exe-subcommand in v2). I don't think this issue was reported to rustup but clap-rs/clap#3693
Add clap_complete as some of clap's functionality was moved into this crate.
Fix related tests.

close #2920

kinnison · 2022-08-27T11:13:49Z

This is a big chunk of change - have you been running this locally too to verify nothing bad happens?

zohnannor · 2022-08-27T19:14:06Z

I hadn't, but I will now, although I am not using rustup directly as often (yes, I know, cargo, rustc and others are rustup's proxies) besides updating nightly and stable Rust installations. I am not 100% sure that everything works besides the functionality that is tested using automatic tests, is there something that is not tested, that I should pay my attention to?

zohnannor · 2022-09-29T23:44:14Z

clap v4 got released, might as well update to that version.

hi-rustin · 2022-10-11T02:21:17Z

This is a big chunk of change - have you been running this locally too to verify nothing bad happens?

@kinnison What kind of tests do we need to do? I am glad to help review and test this PR.

src/cli/rustup_mode.rs

hi-rustin · 2022-10-11T03:02:57Z

Thanks for working on this!

zohnannor · 2022-10-12T23:30:19Z

Still working on transition to v4, will be ready in a couple of days.

UPD: Forgot I can convert PR to draft.

hi-rustin · 2022-10-13T02:04:23Z

Still working on transition to v4, will be ready in a couple of days.

I'd like to bump it up to v3 first and do more tests and release it. Because if we bump it up to v4, then we maybe get more broken changes.

src/cli/rustup_mode.rs

rbtcollins · 2022-12-08T18:56:38Z

This doesn't bring joy :/ ``` OPTIONS: -h, --help Print help information -V, --version Print version information FLAGS: -v, --verbose Enable verbose output -q, --quiet Disable progress output ```

…

On Thu, 8 Dec 2022 at 17:26, zohnannor ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/cli/rustup_mode.rs <#3064 (comment)>: > .version(common::version()) .about("The Rust toolchain installer") .after_help(RUSTUP_HELP) - .setting(AppSettings::VersionlessSubcommands) - .setting(AppSettings::DeriveDisplayOrder) - .setting(AppSettings::SubcommandRequiredElseHelp) + .global_setting(AppSettings::DeriveDisplayOrder) + .arg_required_else_help(true) + .next_help_heading("FLAGS") @@ -1,16 +1,18 @@-$ rustup --help+$ ./target/debug/rustup --help-rustup 1.25.1 (bb60b1e 2022-07-12)+rustup unknown (66def54 2022-10-15) dirty 3 modifications The Rust toolchain installer USAGE:- rustup [FLAGS] [+toolchain] <SUBCOMMAND>+ rustup [OPTIONS] [+toolchain] [SUBCOMMAND]++OPTIONS:+ -h, --help Print help information+ -V, --version Print version information FLAGS: -v, --verbose Enable verbose output -q, --quiet Disable progress output- -h, --help Prints help information- -V, --version Prints version information ARGS: <+toolchain> release channel (e.g. +stable) or custom toolchain to set override@@ -31,7 +33,7 @@ SUBCOMMANDS: self Modify the rustup installation set Alter rustup settings completions Generate tab-completion scripts for your shell- help Prints this message or the help of the given subcommand(s)+ help Print this message or the help of the given subcommand(s) DISCUSSION: Rustup installs The Rust Programming Language from the official@@ -43,33 +45,35 @@ DISCUSSION: If you are new to Rust consider running `rustup doc --book` to learn Rust. -- snip -- Old vs New Old (`rustup 1.25 stable`) New (this `PR` 66def54 <66def54> ) rustup 1.25.1 (bb60b1e 2022-07-12) The Rust toolchain installer USAGE: rustup [FLAGS] [+toolchain] <SUBCOMMAND> FLAGS: -v, --verbose Enable verbose output -q, --quiet Disable progress output -h, --help Prints help information -V, --version Prints version information ARGS: <+toolchain> release channel (e.g. +stable) or custom toolchain to set override SUBCOMMANDS: show Show the active and installed toolchains or profiles update Update Rust toolchains and rustup check Check for updates to Rust toolchains and rustup default Set the default toolchain toolchain Modify or query the installed toolchains target Modify a toolchain's supported targets component Modify a toolchain's installed components override Modify directory toolchain overrides run Run a command with an environment configured for a given toolchain which Display which binary will be run for a given command doc Open the documentation for the current toolchain man View the man page for a given command self Modify the rustup installation set Alter rustup settings completions Generate tab-completion scripts for your shell help Prints this message or the help of the given subcommand(s) DISCUSSION: Rustup installs The Rust Programming Language from the official release channels, enabling you to easily switch between stable, beta, and nightly compilers and keep them updated. It makes cross-compiling simpler with binary builds of the standard library for common platforms. rustup unknown (66def54 2022-10-15) dirty 3 modifications The Rust toolchain installer USAGE: rustup [OPTIONS] [+toolchain] [SUBCOMMAND] OPTIONS: -h, --help Print help information -V, --version Print version information FLAGS: -v, --verbose Enable verbose output -q, --quiet Disable progress output ARGS: <+toolchain> release channel (e.g. +stable) or custom toolchain to set override SUBCOMMANDS: show Show the active and installed toolchains or profiles update Update Rust toolchains and rustup check Check for updates to Rust toolchains and rustup default Set the default toolchain toolchain Modify or query the installed toolchains target Modify a toolchain's supported targets component Modify a toolchain's installed components override Modify directory toolchain overrides run Run a command with an environment configured for a given toolchain which Display which binary will be run for a given command doc Open the documentation for the current toolchain man View the man page for a given command self Modify the rustup installation set Alter rustup settings completions Generate tab-completion scripts for your shell help Print this message or the help of the given subcommand(s) DISCUSSION: Rustup installs The Rust Programming Language from the official release channels, enabling you to easily switch between stable, beta, and nightly compilers and keep them updated. It makes cross-compiling simpler with binary builds of the standard library for common platforms. — Reply to this email directly, view it on GitHub <#3064 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADZ7XWGRHQLIF23NMTEGOLWMID4NANCNFSM57FWENSQ> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

jyn514 · 2023-02-03T03:23:39Z

If it helps, the clap migration guide recommends https://docs.rs/trycmd/latest/trycmd/ for testing the help output. To make it easier to see the differences, you could create a commit only adding trycmd, then upgrade clap, then bless the tests - the changes in the help output will show up in the diff, instead of having to reconstruct them from the code changes.

rbtcollins · 2023-02-04T06:39:49Z

I think that sort of test would be nice.

That said, the main thing is that the new UI will still be usable.

I think my main issue is that the cmd line help syntax line no longer matches the cmd line sections listed below it.

hi-rustin · 2023-02-08T02:45:34Z

If it helps, the clap migration guide recommends https://docs.rs/trycmd/latest/trycmd/ for testing the help output. To make it easier to see the differences, you could create a commit only adding trycmd, then upgrade clap, then bless the tests - the changes in the help output will show up in the diff, instead of having to reconstruct them from the code changes.

@zohnannor Do you have time to add this test? If you are busy right now, I can help to add this test first. I think this test is not only useful for this time, but also we can use it in the future upgrade test.

zohnannor · 2023-02-08T08:29:11Z

@hi-rustin yes, pretty busy right now, I'll try to come back to this Soon™.

@jyn514 to clarify, you're suggesting to use trycmd just to make a diff between old and a new output and then remove trycmd or to integrate it in the project's tests suite and leave it there? I could do either. Before that, to see the output diff, I made a little script that wrote output of every $ rustup <SUBCOMMAND> --help to a file and used that on stable rustup and on the new one, then git-diff'ed it :^)

jyn514 · 2023-02-08T14:51:55Z

@jyn514 to clarify, you're suggesting to use trycmd just to make a diff between old and a new output and then remove trycmd or to integrate it in the project's tests suite and leave it there? I could do either. Before that, to see the output diff, I made a little script that wrote output of every $ rustup <SUBCOMMAND> --help to a file and used that on stable rustup and on the new one, then git-diff'ed it :^)

That's up to the rustup maintainers, but I'd personally recommend integrating it with the test suite and leaving it there, that will make #3094 easier too.

hi-rustin · 2023-02-11T03:37:35Z

If you are busy right now, I can help to add this test first. I think this test is not only useful for this time, but also we can use it in the future upgrade test.

@hi-rustin yes, pretty busy right now, I'll try to come back to this Soon™.

I am working on adding the trycmd tests.

hi-rustin · 2023-02-23T02:18:12Z

@zohnannor Friendly ping~
Do you have any time to work on this PR? Because I was planning to do a release recently. So if we could get this change out that would be nice. Also, if you don't have time to work on this PR, I can help push your PR forward. I can squash your commits. And try to resolve the conflicts and fix all the tests, if you don't mind. I'm very sorry it took so long to go back and forth to get you to change this PR.

Thanks! 💚 💙 💜 💛 ❤️

zohnannor · 2023-02-23T16:32:17Z

Hey @hi-rustin thank you for caring!

I'm very sorry it took so long to go back and forth to get you to change this PR.

No worries!

I was planning to do a release recently.

May I ask for the deadline date?

I definitely will have time in the next couple (and by couple I mean ~4) of days to work on this again! I want to clarify some details of the changes to CLI API, the main complication for us is timezones :) So soon I'll compile a list of details and ask you to correct some of them if something is not as desired.

rbtcollins · 2023-02-23T18:11:16Z

I don't know when @hi-rustin was planning a release, but I can say that I have a thing that won't be ready before the weekend, that I really would love to include in the next release. So IMO the time frames are very compatible.

hi-rustin · 2023-02-24T01:11:34Z

May I ask for the deadline date?

My original plan was to prepare a release in the first week of next month.

hi-rustin · 2023-02-24T01:14:34Z

May I ask for the deadline date?

My original plan was to prepare a release in the first week of next month.

I definitely will have time in the next couple (and by couple I mean ~4) of days to work on this again!

It's would be nice. Thank you very much.

So IMO the time frames are very compatible

Of course. Sounds good. I will wait for those changes. Thanks!

zohnannor · 2023-02-26T17:49:15Z

Hi. I got heavily sick, my plan of finishing this PR in the next couple of days is busted. @hi-rustin you can take and finish it like you said. Thanks for help, very sad I couldn't finish it myself. I still have a #3094 though :)

Sorry again for inconvenience.

hi-rustin · 2023-02-28T00:56:13Z

Hi. I got heavily sick, my plan of finishing this PR in the next couple of days is busted.

Ah, I hope you get well soon.

Sorry again for inconvenience.

No worry. Thanks for your contribution!

hi-rustin

Looks good to me!

After this PR lands, I'll retest it.

rbtcollins

this might need a rebase or something, some of the diff is a bit odd:

   t.skip("tests/cli-ui/rustup/rustup_man_cmd_help_flag_stdout.toml");

  t.skip("tests/suite/cli-ui/rustup/rustup_man_cmd_help_flag_stdout.toml");

src/cli/rustup_mode.rs

hi-rustin · 2023-03-03T13:23:37Z

this might need a rebase or something, some of the diff is a bit odd:

We just miss it last time. So I changed it in this PR. You can see https://github.com/rust-lang/rustup/blob/master/tests/suite/cli_ui.rs

rbtcollins

I'm curious about the new blank line after commands, might make for a little unneeded vertical whitespace on terminals, but nothing to prevent merging.

hi-rustin · 2023-03-04T02:21:44Z

I opened a question to ask the clap community. See clap-rs/clap#4745

hi-rustin

Check again

hi-rustin · 2023-03-05T01:34:00Z

I am going to merge this PR right now. @rbtcollins Thanks for your review! @zohnannor Thanks for your contribution!

This was referenced Sep 15, 2022

Help text for rustup update contains a broken URL when line-broken #2920

Closed

Let Clap wrap help texts automatically #3070

Merged

hi-rustin reviewed Oct 11, 2022

View reviewed changes

src/cli/rustup_mode.rs Outdated Show resolved Hide resolved

src/cli/rustup_mode.rs Outdated Show resolved Hide resolved

src/cli/rustup_mode.rs Outdated Show resolved Hide resolved

zohnannor marked this pull request as draft October 12, 2022 23:34

zohnannor marked this pull request as ready for review October 15, 2022 12:50

zohnannor marked this pull request as draft October 15, 2022 12:51

zohnannor marked this pull request as ready for review October 15, 2022 15:06

zohnannor commented Oct 15, 2022

View reviewed changes

src/cli/rustup_mode.rs Outdated Show resolved Hide resolved

zohnannor commented Oct 15, 2022

View reviewed changes

src/cli/rustup_mode.rs Show resolved Hide resolved

zohnannor commented Oct 15, 2022

View reviewed changes

src/cli/rustup_mode.rs Outdated Show resolved Hide resolved

zohnannor commented Oct 15, 2022

View reviewed changes

src/cli/rustup_mode.rs Outdated Show resolved Hide resolved

zohnannor requested a review from hi-rustin October 15, 2022 15:57

zohnannor mentioned this pull request Oct 15, 2022

Bump clap to 4.0 #3094

Closed

hi-rustin mentioned this pull request Feb 11, 2023

Add test for cli ui #3205

Merged

Bump clap to 3.0

4820b9e

hi-rustin marked this pull request as draft February 28, 2023 01:45

Fix broken test path

aa27cd7

hi-rustin marked this pull request as ready for review March 3, 2023 09:01

hi-rustin added 2 commits March 3, 2023 17:27

Update setup mode tests

55c6fdb

Update rustup mode tests

7e4656a

hi-rustin approved these changes Mar 3, 2023

View reviewed changes

rbtcollins requested changes Mar 3, 2023

View reviewed changes

src/cli/rustup_mode.rs Show resolved Hide resolved

Make match statements better

795f9ef

hi-rustin requested a review from rbtcollins March 3, 2023 15:19

rbtcollins approved these changes Mar 3, 2023

View reviewed changes

Remove extra new line after help

dae8b6a

hi-rustin approved these changes Mar 4, 2023

View reviewed changes

hi-rustin merged commit affbdfe into rust-lang:master Mar 5, 2023

zohnannor deleted the bump-clap branch March 5, 2023 19:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bump `clap` to 3.0 #3064

Bump `clap` to 3.0 #3064

zohnannor commented Aug 21, 2022 •

edited by hi-rustin

kinnison commented Aug 27, 2022

zohnannor commented Aug 27, 2022 •

edited

zohnannor commented Sep 29, 2022

hi-rustin commented Oct 11, 2022 •

edited

hi-rustin commented Oct 11, 2022

zohnannor commented Oct 12, 2022 •

edited

hi-rustin commented Oct 13, 2022

rbtcollins commented Dec 8, 2022 via email

jyn514 commented Feb 3, 2023

rbtcollins commented Feb 4, 2023

hi-rustin commented Feb 8, 2023

zohnannor commented Feb 8, 2023

jyn514 commented Feb 8, 2023 •

edited

hi-rustin commented Feb 11, 2023

hi-rustin commented Feb 23, 2023 •

edited

zohnannor commented Feb 23, 2023

rbtcollins commented Feb 23, 2023

hi-rustin commented Feb 24, 2023

hi-rustin commented Feb 24, 2023

zohnannor commented Feb 26, 2023

hi-rustin commented Feb 28, 2023

hi-rustin left a comment

rbtcollins left a comment

hi-rustin commented Mar 3, 2023

rbtcollins left a comment

hi-rustin commented Mar 4, 2023

hi-rustin left a comment

hi-rustin commented Mar 5, 2023

Bump clap to 3.0 #3064

Bump clap to 3.0 #3064

Conversation

zohnannor commented Aug 21, 2022 • edited by hi-rustin

kinnison commented Aug 27, 2022

zohnannor commented Aug 27, 2022 • edited

zohnannor commented Sep 29, 2022

hi-rustin commented Oct 11, 2022 • edited

hi-rustin commented Oct 11, 2022

zohnannor commented Oct 12, 2022 • edited

hi-rustin commented Oct 13, 2022

rbtcollins commented Dec 8, 2022 via email

jyn514 commented Feb 3, 2023

rbtcollins commented Feb 4, 2023

hi-rustin commented Feb 8, 2023

zohnannor commented Feb 8, 2023

jyn514 commented Feb 8, 2023 • edited

hi-rustin commented Feb 11, 2023

hi-rustin commented Feb 23, 2023 • edited

zohnannor commented Feb 23, 2023

rbtcollins commented Feb 23, 2023

hi-rustin commented Feb 24, 2023

hi-rustin commented Feb 24, 2023

zohnannor commented Feb 26, 2023

hi-rustin commented Feb 28, 2023

hi-rustin left a comment

Choose a reason for hiding this comment

rbtcollins left a comment

Choose a reason for hiding this comment

hi-rustin commented Mar 3, 2023

rbtcollins left a comment

Choose a reason for hiding this comment

hi-rustin commented Mar 4, 2023

hi-rustin left a comment

Choose a reason for hiding this comment

hi-rustin commented Mar 5, 2023

Bump `clap` to 3.0 #3064

Bump `clap` to 3.0 #3064

zohnannor commented Aug 21, 2022 •

edited by hi-rustin

zohnannor commented Aug 27, 2022 •

edited

hi-rustin commented Oct 11, 2022 •

edited

zohnannor commented Oct 12, 2022 •

edited

jyn514 commented Feb 8, 2023 •

edited

hi-rustin commented Feb 23, 2023 •

edited